-
-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change base image for Dockerfile #2289
base: main
Are you sure you want to change the base?
Conversation
(cherry picked from commit 553641f)
Default user and group id is now 977 as 999 was taken. !!**This may cause permissions issues in existing installations**. These issues can be fixed by either: - mounting the volume with another container as a root user and changing ownership of `/app/data` (including their contents) to 977:977 - `chmod -R 977:977 /var/lib/docker/volumes/${VOLUME_NAME_OR_HASH}` of the volume from the host's volume directory. - `chmod -R 777 /app/data` (don't do this) Dockerfile and entrypoint.sh was changed to run NO application code as a privileged user. When the container is running there is no code executed in a privileged context, as anything requiring privileges is run during build time. Proposed Solution to dzikoysk#2288
Seems ready for review, has been stable in my testing environment. |
Hmm... I'm a bit worried about changing current behaviour - we have quite wide variety of users, even on some exotic setups. That's also the reason why we basically moved a lot of the logic to the entrypoint.sh script. I think it requires deeper investigation, I'll try to check some historical issues. I guess you could also browse them by searching for Dockerfile or entrypoint keywords. |
The main issues that came to mind are #1762, #1200, #1634.
Below is a todo list on testing deployments,
Could be helpful to see if the users from #1657 and #1762 if they got it up and running and for potential testing. |
8adc663
to
23c5f23
Compare
A few questions:
|
One nice thing about temurin is that maybe we could avoid changing user & group ids.
I think we should go for the smallest one as long as it works. If that's possible, I guess it'd be nice to have a build without cds. |
That's on me, Bellsoft's tag page is crazy full, will be pushing no cds on build stage shortly. |
although, using CDS/CRaC features could be something you look into, as it may improve performance (startup time/memory usage). CRaC would most likely need explicit support from reposilite, javalin, and but CDS (& AppCDS/Dynamic CDS) could be a free performance boost at startup. (& help with memory? tbh idk exactly how CDS works, but the website I linked said it could help with memory) at the cost of a slightly larger docker image (tbh I wonder if the CDS archive files can be bundled into the jar, so that everyone can benefit from it) |
I have created an image with static CDS |
I'd say this is completely out of scope for now to be fair - we should focus on providing smooth migration first. It'd be great if you could quickly check if using these Temurin images would still require changes in permissions on our side. If no, then maybe we should use it for now. If we'd still need similar changes with Temurin tho, I guess we could proceed with Liberica for Reposilite 3.6.0, since you've already made these changes. |
All Alpine based images will have a ping group for group 999.
If we change to a ubuntu/debian based image such as |
The slim image we're using right now is based on debian, so maybe let's do it the following way:
|
Seems like a pretty good migration strategy, I'll write up changes to the scripts and make a PR, I'll rename this one and maybe return it to draft status. |
Caution
!!This may cause permissions issues in existing installations.
The above mentioned permissions issues can be fixed by either:
/app/data
(including their contents) to 977:977chmod -R 977:977 /var/lib/docker/volumes/${VOLUME_NAME_OR_HASH}
of the volume from the host's volume directory.chmod -R 777 /app/data
(don't do this)Proposed fix #2288